Skip to content

Fix terminal process leak when closing the window.#2723

Merged
code-asher merged 2 commits intocoder:masterfrom
outermeasure:master
Feb 22, 2021
Merged

Fix terminal process leak when closing the window.#2723
code-asher merged 2 commits intocoder:masterfrom
outermeasure:master

Conversation

@outermeasure
Copy link
Copy Markdown
Contributor

@outermeasure outermeasure commented Feb 12, 2021

Calling this.process.dispose(); before this.process.shutdown(true); sets a flag _isDisposed to true.
This causes the _kill() procedure to exit early essentially making this.process.shutdown(true) a no op.

It's necessary to await on the shutdown because _kill() is an async function. Otherwise _isDisposed will be set to true before actually killing the process regardless of the call order of dispose and shutdown.

Impact: Closing the session in the browser should now kill long running processes such as filesystem watchers or services started in the terminal.

@code-asher code-asher self-requested a review February 16, 2021 23:44
Copy link
Copy Markdown
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix and the detailed explanation!

@code-asher code-asher merged commit 271c260 into coder:master Feb 22, 2021
@outermeasure outermeasure deleted the master branch February 22, 2021 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants